Zero-copy encrypt of AEAD application data#10899
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes the TLS 1.2 record send path for AEAD application-data by enabling a zero-copy mode where encryption reads plaintext directly from the caller’s buffer and writes the record ciphertext into the output buffer, avoiding an intermediate memcpy. It also includes two correctness fixes discovered during benchmarking with larger STATIC_BUFFER_LEN values, affecting compile-time validation and DTLS record processing safety.
Changes:
- Add conditional “zero-copy” AEAD encryption in
BuildMessage()for AES-GCM/CCM and ChaCha20-Poly1305 (with config-based exclusions and an opt-out macro). - Fix
STATIC_BUFFER_LENvalidation by replacing a broken preprocessor check with a compile-time static assertion and removing the (unnecessary) upper bound. - Prevent
ShrinkInputBuffer()from moving the input buffer during in-flight record processing by gating shrinking onprocessReplystate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
wolfssl/internal.h |
Replaces invalid #if validation of STATIC_BUFFER_LEN with wc_static_assert() to ensure a usable minimum buffer size. |
src/internal.c |
Implements AEAD zero-copy encryption plumbing in BuildMessage()/EncryptDo() and hardens ShrinkInputBuffer() against mid-record buffer moves. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
retest this please |
|
df65eaf to
c53da5e
Compare
|
retest this please |
1 similar comment
|
retest this please |
1cd4f18 to
b91d5c7
Compare
BuildMessage copied the application data into the output buffer and encrypted it in place. For AEAD suites the cipher input is exactly the plaintext, so Encrypt can read it straight from the caller's buffer and write the record to the output buffer, skipping the copy. - EncryptDo AES-GCM/CCM: input == out keeps the in-place record layout [explicit IV | plaintext | tag space]; input != out means input is the plaintext itself. ChaCha20-Poly1305 already reads only the plaintext from input. Other suites keep the copy: CBC/stream append MAC/pad to the cipher input, CID appends the real content type. - Disabled for WOLFSSL_ASYNC_CRYPT and WOLFSSL_THREADED_CRYPT, where encryption can run after BuildMessage returns, and for debug hooks that expect the in-place layout. Define WOLFSSL_BUILD_MSG_NO_ZERO_COPY to force the old behavior. Measured with examples/benchmark/dtls_bench -z (DTLS 1.2, 1300 byte records, aesni + intelasm): +2.9% AES-256-GCM, +1.0% ChaCha20-Poly1305. The copy was ~3% of send-path CPU in perf. Also fix two bugs found while benchmarking with a larger STATIC_BUFFER_LEN: - The STATIC_BUFFER_LEN bounds check had an unbalanced paren and compared enum constants in a preprocessor #if, where they evaluate to 0, so any user supplied value failed to compile. Keep the lower bound as a static assert. Drop the upper bound: values past the largest record are never used by the record layer but can be useful for memory layout reasons. - ShrinkInputBuffer could run in the middle of record processing via FreeHandshakeResources on the first DTLS app data record, moving the record data and invalidating the idx based bookkeeping. The stale curStartIdx made the more-messages-per-record check dispatch a spurious second message over the AEAD tag bytes, handing garbage app data to the caller and corrupting the buffer state for the next read. Only reachable when the remaining record data fits in the static buffer, which it never does with the default STATIC_BUFFER_LEN. Only shrink between records; the deferred shrink happens at the next wolfSSL_read.
check-source-text requires macros that are used but never defined in the tree to be listed in .wolfssl_known_macro_extras.
b91d5c7 to
1239170
Compare
|
retest this please history lost |
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10899
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
Summary
BuildMessagecopied the application data into the output buffer and encrypted it in place. For AEAD suites the cipher input is exactly the plaintext, soEncryptcan read it straight from the caller's buffer and write the record to the output buffer, skipping the copy.EncryptDoAES-GCM/CCM:input == outkeeps the in-place record layout[explicit IV | plaintext | tag space];input != outmeans input is the plaintext itself. ChaCha20-Poly1305 already reads only the plaintext from input. Other suites keep the copy: CBC/stream append MAC/pad to the cipher input, CID appends the real content type.WOLFSSL_ASYNC_CRYPTandWOLFSSL_THREADED_CRYPT, where encryption can run afterBuildMessagereturns, and for debug hooks that expect the in-place layout. DefineWOLFSSL_BUILD_MSG_NO_ZERO_COPYto force the old behavior.examples/benchmark/dtls_bench -z(DTLS 1.2, 1300 byte records, aesni + intelasm): +2.9% AES-256-GCM, +1.0% ChaCha20-Poly1305. The copy was ~3% of send-path CPU in perf.Also fixes two bugs found while benchmarking with a larger
STATIC_BUFFER_LEN:STATIC_BUFFER_LENbounds check had an unbalanced paren and compared enum constants in a preprocessor#if, where they evaluate to 0, so any user supplied value failed to compile. Keep the lower bound as a static assert. Drop the upper bound: values past the largest record are never used by the record layer but can be useful for memory layout reasons.ShrinkInputBuffercould run in the middle of record processing viaFreeHandshakeResourceson the first DTLS app data record, moving the record data and invalidating the idx based bookkeeping. The stalecurStartIdxmade the more-messages-per-record check dispatch a spurious second message over the AEAD tag bytes, handing garbage app data to the caller and corrupting the buffer state for the next read. Only reachable when the remaining record data fits in the static buffer, which it never does with the defaultSTATIC_BUFFER_LEN. Only shrink between records; the deferred shrink happens at the nextwolfSSL_read.Test plan
examples/benchmark/dtls_bench -zshows no regressionSTATIC_BUFFER_LEN